Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Postgres store tests #1040

Merged

Conversation

babakks
Copy link
Member

@babakks babakks commented Sep 4, 2023

Description

This is a side PR that enables Postgres database store tests, which was skipped due to a missing env var.

Engineering checklist

Check only items that apply

  • Documentation updated
  • Covered by unit tests
  • Covered by integration tests

@kian99
Copy link
Contributor

kian99 commented Sep 4, 2023

@babakks the test is passing because of a change I made a while back. I added an alternative ci.yaml that would run when only workflow files have changed to make editing workflows faster, but that should be removed I think, otherwise in situations like these you can't tell if your CI is still working.

@babakks
Copy link
Member Author

babakks commented Sep 4, 2023

@kian99 Yeah, I figured. I just pushed a dummy commit to trigger the workflow and see if it fails.

@@ -32,7 +32,7 @@ func (s *dbSuite) TestInsertSecret(c *qt.C) {
secret := dbmodel.Secret{}
tx := s.Database.DB.First(&secret)
c.Assert(tx.Error, qt.IsNil)
c.Assert(secret.Time, qt.Equals, testTime)
c.Assert(secret.Time, qt.DeepEquals, testTime)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised these tests weren't running before, I recall running them locally but I guess the CI was skipping them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you set the env then they weren't running locally either?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure they were, I remember going through iterations of them failing and fixing them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that seems to be the reason.

@babakks babakks merged commit b525868 into canonical:feature-rebac Sep 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants